Skip to content

feat(registration): cap faculty selection and replace other faculty with majors - #90

Merged
WilliamTayNZ merged 2 commits into
feat/registration-reworkfrom
feat/85-faculty-major-cascading-selection
Jul 27, 2026
Merged

feat(registration): cap faculty selection and replace other faculty with majors#90
WilliamTayNZ merged 2 commits into
feat/registration-reworkfrom
feat/85-faculty-major-cascading-selection

Conversation

@FinleyNeilson

@FinleyNeilson FinleyNeilson commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Cap faculty checkbox selection at MAX_FACULTIES (2), enforced server-side in actions.ts
  • Remove the "Other" faculty checkbox and otherFaculty free-text field entirely (UI, actions.ts, RegistrationDraft, stripIrrelevantFields)
  • Add an independent majors/specialisation section: up to MAX_MAJORS (4) free-text inputs, each capped at MAX_LENGTHS.major (40 chars)
  • No-JS "add another major" via a new addMajor intent that round-trips through the cookie draft, incrementing majorCount and preserving in-progress field values — mirrors the existing "back" navigation pattern
  • Map majors through to ParsedRegistrationFormSubmission via toParsedSubmission

Closes #85

@WilliamTayNZ WilliamTayNZ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, the faculty cap, addMajor round-trip via majorCount, and preserving in-progress fields across that redirect are all solid, and matches the ticket well. Two things to fix before merging:

1. stripIrrelevantFields doesn't strip majors/majorCount in the returningUoa and non-UoA branches

Only the newUoa branch destructures them out:

} else if (lastPage == "newUoa") {
  const { primaryAffiliation, nonUoaExcerpt, nonUoaPitch, ...stripped } = draftFields;

The returningUoa and else (non-UoA) branches don't include majors/majorCount in their destructured-and-discarded set, so they leak through. Concretely: someone starts the UoA path, types a major or two, goes back, switches to "not a UoA student," submits — their typed majors are still sitting in the cookie and flow into fullDraft for a registration type that shouldn't have them. Worth adding majors/majorCount to the stripped fields in both of the other two branches.

Would also be worth adding a case to the existing "stripIrrelevantFields per branch" tests that sets majors in the draft before checking what survives — none of the current ones would have caught this, since none of them set majors before asserting.

2. MAX_LENGTHS.otherFaculty is still in constants.ts

...

Ticket asked for otherFaculty to be removed from RegistrationDraft/stripIrrelevantFields/NewUoaPage — this is the same cleanup, just missed in constants.ts. For now, just delete it — the faculty enum validation this constant was standing in for will be tracked as a new ticket #92, which will make this fully redundant anyway.

Everything else looks good — parseRegistrationFormData.test.ts is a nice bit of extra coverage too, appreciated 👍

@WilliamTayNZ WilliamTayNZ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

Note there needs to be an additional test in the "stripIrrelevantFields per branch" tests in actions.test.ts, same pattern as the existing three (set majors in the cookie draft, assert it's stripped in the returningUoa and newNonUoa branches, kept in the newUoa branch).

But I will merge this for now and this can be picked up later

@WilliamTayNZ
WilliamTayNZ merged commit 2f9319b into feat/registration-rework Jul 27, 2026
@WilliamTayNZ
WilliamTayNZ deleted the feat/85-faculty-major-cascading-selection branch July 27, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants